-
Notifications
You must be signed in to change notification settings - Fork 583
feat: ValidatorKeystore implementation for high-availability signer #19094
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ab1a508 to
662e326
Compare
662e326 to
9c10b83
Compare
d716b2c to
1882379
Compare
1882379 to
7e10af7
Compare
yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts
Outdated
Show resolved
Hide resolved
yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts
Outdated
Show resolved
Hide resolved
| return false; | ||
| } | ||
| // Re-throw unexpected errors | ||
| throw result.reason; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throwing from inside filter feels weird. I wonder what happens to the stack trace. It might be better to throw new Error('Unexpected error', { cause: result.reason }); to also make sure we're throwing errors
Later edit: the stack trace is maintained and points to the source of the error rather than the filter callback
yarn-project/validator-ha-signer/src/validator_ha_signer.test.ts
Outdated
Show resolved
Hide resolved
| ) { | ||
| this.log = createLogger('validator-ha-signer'); | ||
|
|
||
| if (!config.enabled) { | ||
| if (!config.haSigningEnabled) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be controlled at a higher level (ie. in the validator or the start_node.ts script) not in the constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is basically controlled at the validator factory (.new) but makes sense to have it here as a guard too?
46432ea to
b9212d1
Compare
0d9ba72 to
9d8edd3
Compare
Fixes [A-354](https://linear.app/aztec-labs/issue/A-354/integrate-slash-protection-in-signing-path) Integrates the existing validator HA signer into validator-client via a new `HAKeyStore` wrapper, adds type-safe duty identifiers using discriminated unions, and includes integration testing with multiple validator nodes. ## What's New ### 1. HAKeyStore Implementation **New File**: `yarn-project/validator-client/src/key_store/ha_key_store.ts` Transparent wrapper around `ExtendedValidatorKeyStore` that adds HA coordination: - Drop-in replacement for any `ExtendedValidatorKeyStore` implementation - AUTH_REQUEST duties bypass HA coordination (safe to sign multiple times) - Signs with all addresses concurrently using `Promise.allSettled()`, filtering out already-signed duties ```typescript // With HA protection const sigs = await haKeyStore.signTypedData(typedData, { slot: 100n, blockNumber: 50n, blockIndexWithinCheckpoint: 0, dutyType: DutyType.BLOCK_PROPOSAL, }); // Without HA protection (AUTH_REQUEST) const sigs = await haKeyStore.signTypedData(typedData, { dutyType: DutyType.AUTH_REQUEST, }); ``` ### 2. Type-Safe Duty Identifiers **Modified**: `yarn-project/validator-ha-signer/src/db/types.ts` Replaced optional `blockIndexWithinCheckpoint?: number` with discriminated unions: ```typescript type DutyIdentifier = | BlockProposalDutyIdentifier // MUST have blockIndexWithinCheckpoint >= 0 | OtherDutyIdentifier; // Does NOT have this field interface BlockProposalDutyIdentifier { validatorAddress: EthAddress; slot: SlotNumber; blockIndexWithinCheckpoint: number; // Required dutyType: DutyType.BLOCK_PROPOSAL; } ``` Enforces correctness at compile-time and prevents invalid database primary keys. ### 3. Extended Duty Type Support Added: `CHECKPOINT_PROPOSAL`, `GOVERNANCE_VOTE`, `SLASHING_VOTE`, `AUTH_REQUEST` ### 4. Integration Test **New File**: `yarn-project/validator-client/src/validator.ha.integration.test.ts` E2E test with 5 validators sharing a PostgreSQL database. Tests concurrent block proposals, attestations, checkpoint proposals, and slashing protection. ## Key Changes ### Cleanup Timeout Configuration Made `maxStuckDutiesAgeMs` dynamically computed from actual slot duration: ```typescript const haConfig = { ...config, maxStuckDutiesAgeMs: config.maxStuckDutiesAgeMs ?? epochCache.getL1Constants().slotDuration * 2 * 1000, }; ``` ### SigningContext Now Required All `ValidatorKeyStore` signing methods now require `SigningContext` parameter. Updated `LocalKeyStore`, `NodeKeystoreAdapter`, `Web3SignerKeyStore`, and new `HAKeyStore`. ### Database Schema Added `block_index_within_checkpoint` to primary key to support multiple block proposals per slot: ```sql PRIMARY KEY (validator_address, slot, duty_type, block_index_within_checkpoint) ``` ## Configuration ```bash VALIDATOR_HA_DATABASE_URL=postgresql://user:pass@host:port/db VALIDATOR_HA_SIGNING_ENABLED=true VALIDATOR_HA_NODE_ID=validator-node-1 VALIDATOR_HA_POLLING_INTERVAL_MS=100 # Optional VALIDATOR_HA_SIGNING_TIMEOUT_MS=3000 # Optional VALIDATOR_HA_MAX_STUCK_DUTIES_AGE_MS=144000 # Optional (defaults to 2× slot duration) ``` ## Notes - HA is opt-in via `VALIDATOR_HA_SIGNING_ENABLED=true` - Non-HA validators work unchanged - Lock-free read operations - Atomic lock acquisition using `INSERT ... ON CONFLICT` with retry logic (10ms, 20ms, 30ms backoff) to handle PostgreSQL transaction visibility edge cases in high-concurrency scenarios
540b0be to
9c31789
Compare
Flakey Tests🤖 says: This CI run detected 2 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
Fixes A-354
Integrates the existing validator HA signer into validator-client via a new
HAKeyStorewrapper, adds type-safe duty identifiers using discriminated unions, and includes integration testing with multiple validator nodes.What's New
1. HAKeyStore Implementation
New File:
yarn-project/validator-client/src/key_store/ha_key_store.tsTransparent wrapper around
ExtendedValidatorKeyStorethat adds HA coordination:ExtendedValidatorKeyStoreimplementationPromise.allSettled(), filtering out already-signed duties2. Type-Safe Duty Identifiers
Modified:
yarn-project/validator-ha-signer/src/db/types.tsReplaced optional
blockIndexWithinCheckpoint?: numberwith discriminated unions:Enforces correctness at compile-time and prevents invalid database primary keys.
3. Extended Duty Type Support
Added:
CHECKPOINT_PROPOSAL,GOVERNANCE_VOTE,SLASHING_VOTE,AUTH_REQUEST4. Integration Test
New File:
yarn-project/validator-client/src/validator.ha.integration.test.tsE2E test with 5 validators sharing a PostgreSQL database. Tests concurrent block proposals, attestations, checkpoint proposals, and slashing protection.
Key Changes
Cleanup Timeout Configuration
Made
maxStuckDutiesAgeMsdynamically computed from actual slot duration:SigningContext Now Required
All
ValidatorKeyStoresigning methods now requireSigningContextparameter. UpdatedLocalKeyStore,NodeKeystoreAdapter,Web3SignerKeyStore, and newHAKeyStore.Database Schema
Added
block_index_within_checkpointto primary key to support multiple block proposals per slot:PRIMARY KEY (validator_address, slot, duty_type, block_index_within_checkpoint)Configuration
Notes
VALIDATOR_HA_SIGNING_ENABLED=trueINSERT ... ON CONFLICTwith retry logic (10ms, 20ms, 30ms backoff) to handle PostgreSQL transaction visibility edge cases in high-concurrency scenarios